Skip to content

Comments

[PW_SID:1057146] RISC-V: KVM: Fix use-after-free in kvm_riscv_aia_aplic_has_attr()#1492

Open
linux-riscv-bot wants to merge 1 commit intoworkflow__riscv__fixesfrom
pw1057146
Open

[PW_SID:1057146] RISC-V: KVM: Fix use-after-free in kvm_riscv_aia_aplic_has_attr()#1492
linux-riscv-bot wants to merge 1 commit intoworkflow__riscv__fixesfrom
pw1057146

Conversation

@linux-riscv-bot
Copy link

PR for series 1057146 applied to workflow__riscv__fixes

Name: RISC-V: KVM: Fix use-after-free in kvm_riscv_aia_aplic_has_attr()
URL: https://patchwork.kernel.org/project/linux-riscv/list/?series=1057146
Version: 1

Fuzzer reports a KASAN use-after-free bug triggered by a race
between KVM_HAS_DEVICE_ATTR and KVM_SET_DEVICE_ATTR ioctls on the AIA
device. The root cause is that aia_has_attr() invokes
kvm_riscv_aia_aplic_has_attr() without holding dev->kvm->lock, while
a concurrent aia_set_attr() may call aia_init() under that lock. When
aia_init() fails after kvm_riscv_aia_aplic_init() has succeeded, it
calls kvm_riscv_aia_aplic_cleanup() in its fail_cleanup_imsics path,
which frees both aplic_state and aplic_state->irqs. The concurrent
has_attr path can then dereference the freed aplic->irqs in
aplic_read_pending():
	irqd = &aplic->irqs[irq];   /* UAF here */

KASAN report:
 BUG: KASAN: slab-use-after-free in aplic_read_pending
             arch/riscv/kvm/aia_aplic.c:119 [inline]
 BUG: KASAN: slab-use-after-free in aplic_read_pending_word
             arch/riscv/kvm/aia_aplic.c:351 [inline]
 BUG: KASAN: slab-use-after-free in aplic_mmio_read_offset
             arch/riscv/kvm/aia_aplic.c:406
 Read of size 8 at addr ff600000ba965d58 by task 9498
 Call Trace:
  aplic_read_pending arch/riscv/kvm/aia_aplic.c:119 [inline]
  aplic_read_pending_word arch/riscv/kvm/aia_aplic.c:351 [inline]
  aplic_mmio_read_offset arch/riscv/kvm/aia_aplic.c:406
  kvm_riscv_aia_aplic_has_attr arch/riscv/kvm/aia_aplic.c:566
  aia_has_attr arch/riscv/kvm/aia_device.c:469
 allocated by task 9473:
  kvm_riscv_aia_aplic_init arch/riscv/kvm/aia_aplic.c:583
  aia_init arch/riscv/kvm/aia_device.c:248 [inline]
  aia_set_attr arch/riscv/kvm/aia_device.c:334
 freed by task 9473:
  kvm_riscv_aia_aplic_cleanup arch/riscv/kvm/aia_aplic.c:644
  aia_init arch/riscv/kvm/aia_device.c:292 [inline]
  aia_set_attr arch/riscv/kvm/aia_device.c:334

The patch replaces the actual MMIO read in kvm_riscv_aia_aplic_has_attr()
with a new aplic_mmio_has_offset() that only validates whether the given
offset falls within a known APLIC region, without touching any
dynamically allocated state. This is consistent with the KVM API
documentation for KVM_HAS_DEVICE_ATTR:
  "Tests whether a device supports a particular attribute. A successful
   return indicates the attribute is implemented. It does not necessarily
   indicate that the attribute can be read or written in the device's
   current state."
The upper bounds of each region are taken directly from the
RISC-V AIA specification, so the check is independent of the runtime
values of nr_irqs and nr_words.

This patch both fixes the use-after-free and makes the has_attr
implementation semantically correct.

Fixes: 289a007 ("RISC-V: KVM: Expose APLIC registers as attributes of AIA irqchip")
Signed-off-by: Jiakai Xu <jiakaiPeanut@gmail.com>
Signed-off-by: Jiakai Xu <xujiakai2025@iscas.ac.cn>
Signed-off-by: Linux RISC-V bot <linux.riscv.bot@gmail.com>
@linux-riscv-bot
Copy link
Author

Patch 1: "RISC-V: KVM: Fix use-after-free in kvm_riscv_aia_aplic_has_attr()"
build-rv32-defconfig
Desc: Builds riscv32 defconfig
Duration: 138.44 seconds
Result: PASS

@linux-riscv-bot
Copy link
Author

Patch 1: "RISC-V: KVM: Fix use-after-free in kvm_riscv_aia_aplic_has_attr()"
build-rv64-clang-allmodconfig
Desc: Builds riscv64 allmodconfig with Clang, and checks for errors and added warnings
Duration: 1020.61 seconds
Result: PASS

@linux-riscv-bot
Copy link
Author

Patch 1: "RISC-V: KVM: Fix use-after-free in kvm_riscv_aia_aplic_has_attr()"
build-rv64-gcc-allmodconfig
Desc: Builds riscv64 allmodconfig with GCC, and checks for errors and added warnings
Duration: 1379.10 seconds
Result: PASS

@linux-riscv-bot
Copy link
Author

Patch 1: "RISC-V: KVM: Fix use-after-free in kvm_riscv_aia_aplic_has_attr()"
build-rv64-nommu-k210-defconfig
Desc: Builds riscv64 defconfig with NOMMU for K210
Duration: 26.82 seconds
Result: PASS

@linux-riscv-bot
Copy link
Author

Patch 1: "RISC-V: KVM: Fix use-after-free in kvm_riscv_aia_aplic_has_attr()"
build-rv64-nommu-k210-virt
Desc: Builds riscv64 defconfig with NOMMU for the virt platform
Duration: 27.77 seconds
Result: PASS

@linux-riscv-bot
Copy link
Author

Patch 1: "RISC-V: KVM: Fix use-after-free in kvm_riscv_aia_aplic_has_attr()"
checkpatch
Desc: Runs checkpatch.pl on the patch
Duration: 1.82 seconds
Result: WARNING
Output:

CHECK: Unnecessary parentheses around 'off == APLIC_DOMAINCFG'
#79: FILE: arch/riscv/kvm/aia_aplic.c:535:
+	if ((off == APLIC_DOMAINCFG) ||
+		(off >= APLIC_SOURCECFG_BASE && off < (APLIC_SOURCECFG_BASE + 1023 * 4)) ||
+		(off >= APLIC_SETIP_BASE && off < (APLIC_SETIP_BASE + 32 * 4)) ||
+		(off == APLIC_SETIPNUM) ||
+		(off >= APLIC_CLRIP_BASE && off < (APLIC_CLRIP_BASE + 32 * 4)) ||
+		(off == APLIC_CLRIPNUM) ||
+		(off >= APLIC_SETIE_BASE && off < (APLIC_SETIE_BASE + 32 * 4)) ||
+		(off == APLIC_SETIENUM) ||
+		(off >= APLIC_CLRIE_BASE && off < (APLIC_CLRIE_BASE + 32 * 4)) ||
+		(off == APLIC_CLRIENUM) ||
+		(off == APLIC_SETIPNUM_LE) ||
+		(off == APLIC_SETIPNUM_BE) ||
+		(off == APLIC_GENMSI) ||
+		(off >= APLIC_TARGET_BASE && off < (APLIC_TARGET_BASE + 1203 * 4))
+	)

CHECK: Unnecessary parentheses around 'off == APLIC_SETIPNUM'
#79: FILE: arch/riscv/kvm/aia_aplic.c:535:
+	if ((off == APLIC_DOMAINCFG) ||
+		(off >= APLIC_SOURCECFG_BASE && off < (APLIC_SOURCECFG_BASE + 1023 * 4)) ||
+		(off >= APLIC_SETIP_BASE && off < (APLIC_SETIP_BASE + 32 * 4)) ||
+		(off == APLIC_SETIPNUM) ||
+		(off >= APLIC_CLRIP_BASE && off < (APLIC_CLRIP_BASE + 32 * 4)) ||
+		(off == APLIC_CLRIPNUM) ||
+		(off >= APLIC_SETIE_BASE && off < (APLIC_SETIE_BASE + 32 * 4)) ||
+		(off == APLIC_SETIENUM) ||
+		(off >= APLIC_CLRIE_BASE && off < (APLIC_CLRIE_BASE + 32 * 4)) ||
+		(off == APLIC_CLRIENUM) ||
+		(off == APLIC_SETIPNUM_LE) ||
+		(off == APLIC_SETIPNUM_BE) ||
+		(off == APLIC_GENMSI) ||
+		(off >= APLIC_TARGET_BASE && off < (APLIC_TARGET_BASE + 1203 * 4))
+	)

CHECK: Unnecessary parentheses around 'off == APLIC_CLRIPNUM'
#79: FILE: arch/riscv/kvm/aia_aplic.c:535:
+	if ((off == APLIC_DOMAINCFG) ||
+		(off >= APLIC_SOURCECFG_BASE && off < (APLIC_SOURCECFG_BASE + 1023 * 4)) ||
+		(off >= APLIC_SETIP_BASE && off < (APLIC_SETIP_BASE + 32 * 4)) ||
+		(off == APLIC_SETIPNUM) ||
+		(off >= APLIC_CLRIP_BASE && off < (APLIC_CLRIP_BASE + 32 * 4)) ||
+		(off == APLIC_CLRIPNUM) ||
+		(off >= APLIC_SETIE_BASE && off < (APLIC_SETIE_BASE + 32 * 4)) ||
+		(off == APLIC_SETIENUM) ||
+		(off >= APLIC_CLRIE_BASE && off < (APLIC_CLRIE_BASE + 32 * 4)) ||
+		(off == APLIC_CLRIENUM) ||
+		(off == APLIC_SETIPNUM_LE) ||
+		(off == APLIC_SETIPNUM_BE) ||
+		(off == APLIC_GENMSI) ||
+		(off >= APLIC_TARGET_BASE && off < (APLIC_TARGET_BASE + 1203 * 4))
+	)

CHECK: Unnecessary parentheses around 'off == APLIC_SETIENUM'
#79: FILE: arch/riscv/kvm/aia_aplic.c:535:
+	if ((off == APLIC_DOMAINCFG) ||
+		(off >= APLIC_SOURCECFG_BASE && off < (APLIC_SOURCECFG_BASE + 1023 * 4)) ||
+		(off >= APLIC_SETIP_BASE && off < (APLIC_SETIP_BASE + 32 * 4)) ||
+		(off == APLIC_SETIPNUM) ||
+		(off >= APLIC_CLRIP_BASE && off < (APLIC_CLRIP_BASE + 32 * 4)) ||
+		(off == APLIC_CLRIPNUM) ||
+		(off >= APLIC_SETIE_BASE && off < (APLIC_SETIE_BASE + 32 * 4)) ||
+		(off == APLIC_SETIENUM) ||
+		(off >= APLIC_CLRIE_BASE && off < (APLIC_CLRIE_BASE + 32 * 4)) ||
+		(off == APLIC_CLRIENUM) ||
+		(off == APLIC_SETIPNUM_LE) ||
+		(off == APLIC_SETIPNUM_BE) ||
+		(off == APLIC_GENMSI) ||
+		(off >= APLIC_TARGET_BASE && off < (APLIC_TARGET_BASE + 1203 * 4))
+	)

CHECK: Unnecessary parentheses around 'off == APLIC_CLRIENUM'
#79: FILE: arch/riscv/kvm/aia_aplic.c:535:
+	if ((off == APLIC_DOMAINCFG) ||
+		(off >= APLIC_SOURCECFG_BASE && off < (APLIC_SOURCECFG_BASE + 1023 * 4)) ||
+		(off >= APLIC_SETIP_BASE && off < (APLIC_SETIP_BASE + 32 * 4)) ||
+		(off == APLIC_SETIPNUM) ||
+		(off >= APLIC_CLRIP_BASE && off < (APLIC_CLRIP_BASE + 32 * 4)) ||
+		(off == APLIC_CLRIPNUM) ||
+		(off >= APLIC_SETIE_BASE && off < (APLIC_SETIE_BASE + 32 * 4)) ||
+		(off == APLIC_SETIENUM) ||
+		(off >= APLIC_CLRIE_BASE && off < (APLIC_CLRIE_BASE + 32 * 4)) ||
+		(off == APLIC_CLRIENUM) ||
+		(off == APLIC_SETIPNUM_LE) ||
+		(off == APLIC_SETIPNUM_BE) ||
+		(off == APLIC_GENMSI) ||
+		(off >= APLIC_TARGET_BASE && off < (APLIC_TARGET_BASE + 1203 * 4))
+	)

CHECK: Unnecessary parentheses around 'off == APLIC_SETIPNUM_LE'
#79: FILE: arch/riscv/kvm/aia_aplic.c:535:
+	if ((off == APLIC_DOMAINCFG) ||
+		(off >= APLIC_SOURCECFG_BASE && off < (APLIC_SOURCECFG_BASE + 1023 * 4)) ||
+		(off >= APLIC_SETIP_BASE && off < (APLIC_SETIP_BASE + 32 * 4)) ||
+		(off == APLIC_SETIPNUM) ||
+		(off >= APLIC_CLRIP_BASE && off < (APLIC_CLRIP_BASE + 32 * 4)) ||
+		(off == APLIC_CLRIPNUM) ||
+		(off >= APLIC_SETIE_BASE && off < (APLIC_SETIE_BASE + 32 * 4)) ||
+		(off == APLIC_SETIENUM) ||
+		(off >= APLIC_CLRIE_BASE && off < (APLIC_CLRIE_BASE + 32 * 4)) ||
+		(off == APLIC_CLRIENUM) ||
+		(off == APLIC_SETIPNUM_LE) ||
+		(off == APLIC_SETIPNUM_BE) ||
+		(off == APLIC_GENMSI) ||
+		(off >= APLIC_TARGET_BASE && off < (APLIC_TARGET_BASE + 1203 * 4))
+	)

CHECK: Unnecessary parentheses around 'off == APLIC_SETIPNUM_BE'
#79: FILE: arch/riscv/kvm/aia_aplic.c:535:
+	if ((off == APLIC_DOMAINCFG) ||
+		(off >= APLIC_SOURCECFG_BASE && off < (APLIC_SOURCECFG_BASE + 1023 * 4)) ||
+		(off >= APLIC_SETIP_BASE && off < (APLIC_SETIP_BASE + 32 * 4)) ||
+		(off == APLIC_SETIPNUM) ||
+		(off >= APLIC_CLRIP_BASE && off < (APLIC_CLRIP_BASE + 32 * 4)) ||
+		(off == APLIC_CLRIPNUM) ||
+		(off >= APLIC_SETIE_BASE && off < (APLIC_SETIE_BASE + 32 * 4)) ||
+		(off == APLIC_SETIENUM) ||
+		(off >= APLIC_CLRIE_BASE && off < (APLIC_CLRIE_BASE + 32 * 4)) ||
+		(off == APLIC_CLRIENUM) ||
+		(off == APLIC_SETIPNUM_LE) ||
+		(off == APLIC_SETIPNUM_BE) ||
+		(off == APLIC_GENMSI) ||
+		(off >= APLIC_TARGET_BASE && off < (APLIC_TARGET_BASE + 1203 * 4))
+	)

CHECK: Unnecessary parentheses around 'off == APLIC_GENMSI'
#79: FILE: arch/riscv/kvm/aia_aplic.c:535:
+	if ((off == APLIC_DOMAINCFG) ||
+		(off >= APLIC_SOURCECFG_BASE && off < (APLIC_SOURCECFG_BASE + 1023 * 4)) ||
+		(off >= APLIC_SETIP_BASE && off < (APLIC_SETIP_BASE + 32 * 4)) ||
+		(off == APLIC_SETIPNUM) ||
+		(off >= APLIC_CLRIP_BASE && off < (APLIC_CLRIP_BASE + 32 * 4)) ||
+		(off == APLIC_CLRIPNUM) ||
+		(off >= APLIC_SETIE_BASE && off < (APLIC_SETIE_BASE + 32 * 4)) ||
+		(off == APLIC_SETIENUM) ||
+		(off >= APLIC_CLRIE_BASE && off < (APLIC_CLRIE_BASE + 32 * 4)) ||
+		(off == APLIC_CLRIENUM) ||
+		(off == APLIC_SETIPNUM_LE) ||
+		(off == APLIC_SETIPNUM_BE) ||
+		(off == APLIC_GENMSI) ||
+		(off >= APLIC_TARGET_BASE && off < (APLIC_TARGET_BASE + 1203 * 4))
+	)

CHECK: Alignment should match open parenthesis
#80: FILE: arch/riscv/kvm/aia_aplic.c:536:
+	if ((off == APLIC_DOMAINCFG) ||
+		(off >= APLIC_SOURCECFG_BASE && off < (APLIC_SOURCECFG_BASE + 1023 * 4)) ||

total: 0 errors, 0 warnings, 9 checks, 44 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

Commit b86997f39d4f ("RISC-V: KVM: Fix use-after-free in kvm_riscv_aia_aplic_has_attr()") has style problems, please review.

NOTE: Ignored message types: ALLOC_SIZEOF_STRUCT CAMELCASE COMMIT_LOG_LONG_LINE GIT_COMMIT_ID MACRO_ARG_REUSE NO_AUTHOR_SIGN_OFF

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.
total: 0 errors, 0 warnings, 9 checks, 44 lines checked
CHECK: Alignment should match open parenthesis
CHECK: Unnecessary parentheses around 'off == APLIC_CLRIENUM'
CHECK: Unnecessary parentheses around 'off == APLIC_CLRIPNUM'
CHECK: Unnecessary parentheses around 'off == APLIC_DOMAINCFG'
CHECK: Unnecessary parentheses around 'off == APLIC_GENMSI'
CHECK: Unnecessary parentheses around 'off == APLIC_SETIENUM'
CHECK: Unnecessary parentheses around 'off == APLIC_SETIPNUM'
CHECK: Unnecessary parentheses around 'off == APLIC_SETIPNUM_BE'
CHECK: Unnecessary parentheses around 'off == APLIC_SETIPNUM_LE'


@linux-riscv-bot
Copy link
Author

Patch 1: "RISC-V: KVM: Fix use-after-free in kvm_riscv_aia_aplic_has_attr()"
dtb-warn-rv64
Desc: Checks for Device Tree warnings/errors
Duration: 84.81 seconds
Result: PASS

@linux-riscv-bot
Copy link
Author

Patch 1: "RISC-V: KVM: Fix use-after-free in kvm_riscv_aia_aplic_has_attr()"
header-inline
Desc: Detects static functions without inline keyword in header files
Duration: 0.22 seconds
Result: PASS

@linux-riscv-bot
Copy link
Author

Patch 1: "RISC-V: KVM: Fix use-after-free in kvm_riscv_aia_aplic_has_attr()"
kdoc
Desc: Detects for kdoc errors
Duration: 0.84 seconds
Result: PASS

@linux-riscv-bot
Copy link
Author

Patch 1: "RISC-V: KVM: Fix use-after-free in kvm_riscv_aia_aplic_has_attr()"
module-param
Desc: Detect module_param changes
Duration: 0.24 seconds
Result: PASS

@linux-riscv-bot
Copy link
Author

Patch 1: "RISC-V: KVM: Fix use-after-free in kvm_riscv_aia_aplic_has_attr()"
verify-fixes
Desc: Verifies that the Fixes: tags exist
Duration: 0.27 seconds
Result: PASS

@linux-riscv-bot
Copy link
Author

Patch 1: "RISC-V: KVM: Fix use-after-free in kvm_riscv_aia_aplic_has_attr()"
verify-signedoff
Desc: Verifies that Signed-off-by: tags are correct
Duration: 0.28 seconds
Result: PASS

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants